Skip to content
This repository was archived by the owner on Oct 25, 2021. It is now read-only.

Svetlana_Emelianchik#27

Open
rlrio wants to merge 82 commits intomasterfrom
feature/SvetlanaEmelianchik
Open

Svetlana_Emelianchik#27
rlrio wants to merge 82 commits intomasterfrom
feature/SvetlanaEmelianchik

Conversation

@rlrio
Copy link
Collaborator

@rlrio rlrio commented Jul 9, 2021

No description provided.

@NikolaevArtem NikolaevArtem added the bug Code fix needed label Jul 11, 2021
@NikolaevArtem NikolaevArtem changed the title Feature/svetlana emelianchik Svetlana_Emelianchik Jul 11, 2021
@rlrio rlrio added the readyForReview Sign for Artem to take a look label Jul 12, 2021
@NikolaevArtem NikolaevArtem added HW_1 Good to go and removed bug Code fix needed readyForReview Sign for Artem to take a look labels Jul 13, 2021
@rlrio rlrio added the readyForReview Sign for Artem to take a look label Jul 14, 2021
@NikolaevArtem NikolaevArtem removed the readyForReview Sign for Artem to take a look label Jul 20, 2021
@NikolaevArtem NikolaevArtem removed the readyForReview Sign for Artem to take a look label Sep 21, 2021

public class Main {

public static void main(String[] args) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opt: it seems indentation is a little bit shifted
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
it looks like this on my computer

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice game, well-done! Excellent architecture, model/service separation, easy to read code, pretty interface and a lot of options how to play. Also, you're showing good knowledge of Java Core and Java libraries, as well as following Code Conventions and creating proper application architecture. Special thanks for automatic ship placement :) Testing is also fine, as I understand testing of console application can be tough.

Good job, approved!

Player player2 = players[1];
boolean isGameOver = false;
while (!isGameOver) {
isGameOver = service.isGameOver(player1, player2, gameMode);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opt: service.isGameOver has really unexpected nature - most of game logic is there, but actually, by name, it should only check isGameOver or not, nothing else. Better separate

}

public List<String> getTasks() {
return tasks;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line makes the object mutable

Collections.copy(tasks, this.tasks);
}
if (age == null) {
age = this.getAge();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As well as this

import java.util.ArrayList;
import java.util.List;

public class Main {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Result of Main:
image

package homework_4.custom_annotation;

@Author
public class Book {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good example!


import static java.lang.System.lineSeparator;

public class CustomFileReader {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good!

import java.util.HashMap;
import java.util.Map;

public class Main {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good!


public static void main(String[] args) {
Kitten kitten = new Kitten("Kitty Tom", 1, 2);
Cat grownUpCat = kitten.grow(k -> new Cat(k.getName().replace("Kitty", "Big"), k.getAge() + 1, k.getWeight() * 2));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opt: you could also try type conversion (like, weight to size or smth like this). Means Kitten and Cat have different field by type

Copy link
Owner

@NikolaevArtem NikolaevArtem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! The course project is excellent, minor issues in HW_3, but I think it's easy to fix, so I'll approve now

Approved

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants